Skip to content

Make EnumSet not silently corrupt data. #18721

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 7, 2014

Conversation

SimonSapin
Copy link
Contributor

Assert at run time instead. Fixes #13756.

I’d rather have this be detected at compile-time, but I don’t know how to do that.

@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

@rust-highfive
Copy link
Contributor

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@Gankra
Copy link
Contributor

Gankra commented Nov 6, 2014

Worse, this was actually invoking UB with out current bitshift semantics! Thanks for catching this!

I basically rewrote EnumSet in #18605 to bring it closer to our new Set conventions, you may want to rebase your work on that.

(also if you want to sanity check that that refactor is "right" for this type, I wouldn't mind that!)

@alexcrichton
Copy link
Member

Looks good to me, I've r+'d and this can be rebased once the rollup lands

Assert at run time instead. Fixes rust-lang#13756.

I’d rather have this be detected at compile-time, but I don’t know how to do that.
@SimonSapin
Copy link
Contributor Author

Rebased.

bors added a commit that referenced this pull request Nov 7, 2014
Assert at run time instead. Fixes #13756.

I’d rather have this be detected at compile-time, but I don’t know how to do that.
@bors bors closed this Nov 7, 2014
@bors bors merged commit d8ab2f8 into rust-lang:master Nov 7, 2014
@SimonSapin SimonSapin deleted the safer-enumset branch November 7, 2014 18:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EnumSet with more variants than bits in uint silently corrupts data
5 participants